-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding Accounts Service #3
Conversation
v5/pivotal/stories.go
Outdated
if len(fields) == 0 { | ||
fields = DefaultFields | ||
} | ||
u += "&fields=" + url.QueryEscape(fieldsToQuery(fields)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some info about this,
If we don't explicitly request for some fields like requested_by
, it doesn't return it in the response. However, the use of this field causes only the mentioned fields to appear. So, we have to fill this parameter with all the default fields again.
@@ -0,0 +1,2 @@ | |||
module github.com/oschwald/go-pivotaltracker | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Run go mod tidy
. There should be a Go version number.
return func() *http.Request { | ||
u := fmt.Sprintf("projects/%v/stories", projectID) | ||
if filter != "" { | ||
u += "?filter=" + url.QueryEscape(filter) | ||
if len(fields) == 0 { | ||
fields = DefaultFields | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems redundant given that fieldsToQuery
also sets the default fields.
I do wonder if not including the param would be less surprising if there are no fields passed, but I am fine with it as is given that we are only using this internally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I agree. Thank you, I have removed it. 👍
&fields
parameter. This change lets us get therequested_by
field.go.mod
. This makes it easier for forks to replace the original when working on the work. Otherwise, it throws an error. Ex.replace github.com/oschwald/go-pivotaltracker => Path-to-forked/go-pivotaltracker